(wip) Update vm-operator spec to v1alpha2 #2831

adikul30 · 2024-03-18T23:45:56Z

What this PR does / why we need it:
vm-operator has shifted from API version v1alpha1 to v1alpha2. CSI moving it's client from v1a1 to v1a2 would mean the vm-op conversion webhook won't be involved thus saving time and resources.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Testing done:
#2831 (comment)

Special notes for your reviewer:

Release note:

k8s-ci-robot · 2024-03-18T23:46:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adikul30

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adikul30]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

divyenpatel · 2024-03-19T22:04:04Z

pkg/syncer/cnsoperator/util/util.go

@@ -127,7 +153,7 @@ func GetTKGVMIP(ctx context.Context, vmOperatorClient client.Client, dc dynamic.
 			return "", fmt.Errorf("failed to get SNAT IP annotation from VirtualMachine %s/%s", vmNamespace, vmName)
 		}
 	} else {
-		networkInterfaceName := networkName + "-" + vmName


If we use latest image with this change on the older setup with v1alpha1 API, then are we breaking this logic?

Yes, if this change is run on a setup where VMs are deployed with spec from v1alpha1 then this logic won't work.

VM operator team confirmed offline that in case of upgrades, this conversion webhook https://github.com/vmware-tanzu/vm-operator/blob/ace7d6109146e8350df8e49fac8f8ebbf3141487/api/v1alpha1/virtualmachine_conversion.go#L29 will handle converting CRDs from v1alpha1 to v1alpha2 so we don't have to worry about such a scenario.

The interface name pattern is not something about which y'all should care. In VM Op this depends on a myriad of factors. Why are y'all using it?

This knowledge should not be here. The prior or current network interface CR naming convention is not anything sort of contract. This also has assumptions about how CAPV works today by not doing defaulting via an empty NetworkName, not using DHCP, and is broke for the upcoming VCP work since that uses a new CRD.

This isn't something that the conversion webhook matters here because the underlying network interface CR has a different naming convention with the v1a2 work because the Spec.Network.Interfaces[] now has a Name field.

Why isn't this using what's in the VM.Status?

divyenpatel · 2024-03-19T22:28:54Z

/ok-to-test

bryanv · 2024-03-21T22:08:42Z

tests/e2e/vmservice_utils.go

@@ -338,10 +346,10 @@ func waitNgetVmsvcVmIp(ctx context.Context, c ctlrclient.Client, namespace strin
 			}
 			return false, nil
 		}
-		if vm.Status.VmIp == "" {
+		if vm.Status.Network.PrimaryIP4 == "" {


vm.Status.Network is a pointer that can be nil

bryanv · 2024-03-21T22:10:30Z

tests/e2e/vmservice_utils.go

 	"golang.org/x/crypto/ssh"

-	vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1"
+	vmopv2 "github.com/vmware-tanzu/vm-operator/api/v1alpha2"


Suggestion: keep your import alias as vmopv1 `cause it is still v1 and the diff is smaller. For whenever we get around to a v1a3, it will be an small, incremental change from v1a2 so most likely the only change required here will be to update the import.

adikul30 · 2024-04-10T17:17:16Z

Build #1375 (Apr 9, 2024, 4:13:53 PM)
adkulkarni
PR 2831
Ran 13 of 816 Specs in 2659.499 seconds SUCCESS! -- 13 Passed | 0 Failed | 0 Pending | 803 Skipped PASS Ginkgo ran 1 suite in 46m6.024409434s Test Suite Passed make: Leaving directory `/home/worker/workspace/csi-wcp-precheckin/Results/1375/vsphere-csi-driver'

adikul30 · 2024-04-10T17:19:12Z

cc: @nikhilbarge

k8s-ci-robot · 2024-05-22T00:27:41Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-triage-robot · 2024-08-20T00:33:22Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2024-09-19T01:32:21Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle rotten
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot · 2024-10-19T02:15:53Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen
Mark this PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2024-10-19T02:15:58Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Reopen this PR with /reopen

Mark this PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot requested review from gnufied and kavyashree-r March 18, 2024 23:46

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 18, 2024

adikul30 changed the title ~~Update vm-operator spec to v1alpha2~~ (wip) Update vm-operator spec to v1alpha2 Mar 18, 2024

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 18, 2024

adikul30 force-pushed the topic/adkulkarni/vmspec-v1a2 branch 5 times, most recently from cdac8da to b3403f7 Compare March 19, 2024 17:46

adikul30 changed the title ~~(wip) Update vm-operator spec to v1alpha2~~ Update vm-operator spec to v1alpha2 Mar 19, 2024

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 19, 2024

divyenpatel reviewed Mar 19, 2024

View reviewed changes

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 19, 2024

bryanv reviewed Mar 21, 2024

View reviewed changes

adikul30 force-pushed the topic/adkulkarni/vmspec-v1a2 branch 2 times, most recently from 7c1eb3f to 0a3698e Compare March 23, 2024 00:17

adikul30 mentioned this pull request Mar 25, 2024

Fix GetTKGVMIP() logic in VDS for vm-op v1alpha1 #2838

Merged

adikul30 force-pushed the topic/adkulkarni/vmspec-v1a2 branch 2 times, most recently from 2c21b4c to 81f4143 Compare March 25, 2024 22:33

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 9, 2024

Update vm-operator spec to v1alpha2

76eee40

adikul30 force-pushed the topic/adkulkarni/vmspec-v1a2 branch from 81f4143 to 76eee40 Compare April 9, 2024 20:58

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 9, 2024

adikul30 requested a review from divyenpatel April 10, 2024 17:18

adikul30 changed the title ~~Update vm-operator spec to v1alpha2~~ (wip) Update vm-operator spec to v1alpha2 Apr 10, 2024

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 10, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2024

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 20, 2024

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 19, 2024

k8s-ci-robot closed this Oct 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(wip) Update vm-operator spec to v1alpha2 #2831

(wip) Update vm-operator spec to v1alpha2 #2831

adikul30 commented Mar 18, 2024 •

edited

Loading

k8s-ci-robot commented Mar 18, 2024

divyenpatel Mar 19, 2024

adikul30 Mar 19, 2024

adikul30 Mar 19, 2024

akutz Mar 21, 2024

akutz Mar 21, 2024

bryanv Mar 21, 2024

divyenpatel commented Mar 19, 2024

bryanv Mar 21, 2024

adikul30 Mar 23, 2024

bryanv Mar 21, 2024

adikul30 Mar 23, 2024

adikul30 commented Apr 10, 2024

adikul30 commented Apr 10, 2024

k8s-ci-robot commented May 22, 2024

k8s-triage-robot commented Aug 20, 2024

k8s-triage-robot commented Sep 19, 2024

k8s-triage-robot commented Oct 19, 2024

k8s-ci-robot commented Oct 19, 2024

(wip) Update vm-operator spec to v1alpha2 #2831

(wip) Update vm-operator spec to v1alpha2 #2831

Conversation

adikul30 commented Mar 18, 2024 • edited Loading

k8s-ci-robot commented Mar 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divyenpatel commented Mar 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adikul30 commented Apr 10, 2024

adikul30 commented Apr 10, 2024

k8s-ci-robot commented May 22, 2024

k8s-triage-robot commented Aug 20, 2024

k8s-triage-robot commented Sep 19, 2024

k8s-triage-robot commented Oct 19, 2024

k8s-ci-robot commented Oct 19, 2024

adikul30 commented Mar 18, 2024 •

edited

Loading